Decrease time-to-sync for learn-only devices joining the network #11271
+90
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
DEV: backend
Build Artifacts
|
bjester
requested changes
Sep 21, 2023
bjester
approved these changes
Sep 26, 2023
| mock_enqueue.assert_called_once_with( | ||
| job_id="9e89d3ea5256721c9cd631eac36feafe", | ||
| args=(self.mock_broadcast.id, self.instance.to_dict()), | ||
| args=(self.broadcast.id, self.instance.to_dict()), | ||
| priority=mock_priority_method(), |
There was a problem hiding this comment.
using mock_priority_method.return_value can be a bit clearer for test assertions, and also as a practice avoids potential gotchas asserting the number of times a mock is called.
| }, | ||
| ) | ||
|
|
||
| self.broadcast = KolibriBroadcast(instance=self.broadcast_instance) |
There was a problem hiding this comment.
I would prefer to keep using the mock, to keep the unit testing scope limited to the listeners. You can use spec or spec_set on the mock to constrain its usage to align with a particular class, in this case KolibriBroadcast. All that said, the broadcast class doesn't do much without start_broadcast called.
|
@bjester sir will keep in mind those suggestions for future test creations. Appreciate the detailed review sir, helps me improve ❤️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Decreases the time-to-sync for learn-only devices joining the network by playing with some task's priorities.
Developed together with @akolson ❤️
References
Closes #11220.
Reviewer guidance
Make sure the if condition is as specified in #11220.
Testing checklist
PR process
Reviewer checklist
yarnandpip)